Skip to content

Conversation

@petrochenkov
Copy link
Contributor

This is a re-implementation of #144131 with all the issues mentioned there fixed.

As it turned out, the non-trivial part of the split was already done in c91b6ca, so the remaining part implemented in this PR is mostly mechanical.

After addressing the issue of already found bindings being lost due to indeterminacies in outer scopes (7e890bf) and adding one missing Stage::Late in Finalize the scope splitting refactoring just worked.
(One more ICE was revealed by the refactoring, but not caused by it, fixed up in the last commit.)

This is a part of implementation for the Open API proposal.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 5, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@petrochenkov
Copy link
Contributor Author

There may be minor breakage in corner cases due to ambiguity checking being unified between in-scope resolution and in-module resolution, so this needs a crater run.
@bors try

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Dec 5, 2025
resolve: Split `Scope::Module` into two scopes for non-glob and glob bindings
@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2025
&& glob_binding.res() != non_glob_binding.res()
{
resolution.non_glob_binding = Some(this.new_ambiguity_binding(
AmbiguityKind::GlobVsExpanded,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yaahc you may be interested, this PR removes one of the "gray areas".

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment was marked as resolved.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 6, 2025
@rustbot

This comment has been minimized.

@petrochenkov
Copy link
Contributor Author

@bors try

rust-bors bot added a commit that referenced this pull request Dec 6, 2025
resolve: Split `Scope::Module` into two scopes for non-glob and glob bindings
@rust-bors

This comment has been minimized.

@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 6, 2025
@rust-bors
Copy link

rust-bors bot commented Dec 6, 2025

☀️ Try build successful (CI)
Build commit: 0c70cfb (0c70cfbec949d241375e31c5a42066be70c720fc, parent: da2544bfbe84df7b24d83c029c74299ebf6112c6)

@petrochenkov
Copy link
Contributor Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-149681 created and queued.
🤖 Automatically detected try build 0c70cfb
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'm not incredibly familiar with name resolution, so you might want another review, but if this is mostly mechanical then it's up to you

View changes since this review

@yaahc yaahc added the A-resolve Area: Name/path resolution done by `rustc_resolve` specifically label Dec 9, 2025
@bors
Copy link
Collaborator

bors commented Dec 13, 2025

☔ The latest upstream changes (presumably #149941) made this pull request unmergeable. Please resolve the merge conflicts.

@craterbot
Copy link
Collaborator

🚧 Experiment pr-149681 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-149681 is completed!
📊 58 regressed and 5 fixed (753932 total)
📊 2039 spurious results on the retry-regressed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-149681/retry-regressed-list.txt

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 21, 2025
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 21, 2025
…d the innermost binding.

Previously we could lose the already found binding and break with an error, if some blocking error was found in the shadowed scopes.

Also, avoid some impossible state in the return type of `fn resolve_ident_in_scope`.
in the same module to the usual ambiguity infra in `resolve_ident_in_scope_set`
for looking up a name in two scopes insice a module - non-glob and glob bindings.
…ings

in the same module to the usual ambiguity infra in `resolve_ident_in_scope_set`
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Dec 21, 2025

The remaining 4 root regressions are similar to #145575 that happened because of the similar prelude scope split, and can be mitigated with a similar hack.
I'll implement it during the week.

@collinfounier346523-oss
Copy link

@rustbot
Copy link
Collaborator

rustbot commented Dec 21, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-resolve Area: Name/path resolution done by `rustc_resolve` specifically S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants